size-bound 50 MB packs by default, fix stale chunk index after compact pack rewrites - #9852
Merged
ThomasWaldmann merged 3 commits intoJul 3, 2026
Conversation
max_count=3 was a testing value. The default is now max_count=None, max_size=50_000_000; the testsuite sets BORG_PACK_MAX_COUNT=3 instead, so small test data still produces multiple multi-object packs.
…orgbackup#9850 compact_pack() updated repository.chunks, but compact computes keep/drop from and persists its own index, which kept pointing at the deleted packs. Also: tolerate deleting already-gone packs, start rebuilt index entries as unused.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9852 +/- ##
=======================================
Coverage 85.11% 85.11%
=======================================
Files 93 93
Lines 15388 15409 +21
Branches 2322 2326 +4
=======================================
+ Hits 13097 13115 +18
- Misses 1592 1596 +4
+ Partials 699 698 -1 ☔ View full report in Codecov by Harness. |
ThomasWaldmann
requested changes
Jul 3, 2026
ThomasWaldmann
left a comment
ThomasWaldmann
left a comment
Member
There was a problem hiding this comment.
good you found that bug!
some nitpicks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two changes, one commit each.
Pack sizing defaults (fixes #9843)
Packs are now bound by size:
max_size=50_000_000,max_count=None. The oldmax_count=3was a testing value. The testsuite setsBORG_PACK_MAX_COUNT=3via theclean_envfixture, so small test data still produces multiple multi-object packs and the rollover and compaction code paths stay covered.compact left a stale chunk index after pack rewrites (fixes #9850)
I traced the
ObjectNotFoundcrash from #9850 to this:borg compactcomputes its keep/drop sets from its own chunk index, butcompact_pack()applied the resulting updates torepository.chunks, a different index object.save_chunk_index()then persisted the unchanged index, so after every mixed-pack rewrite the persisted entries still pointed at the deleted old pack. Once a laterborg deletemade all of that phantom pack's entries unused, the next compact classified it as droppable and crashed trying to delete a pack file that was already gone.No data is lost in this situation: the kept objects were copied into the new pack, only the persisted pointers were stale. Rebuilding the index from the packs (deleting the
index/*objects) repairs an affected repository.Fixes:
compact_pack()takes achunksparameter and compact passes the index it computed keep/drop from, so the updates land in the index that gets persisted. As a side effect this removes a full slow index rebuild that happened mid-compact, whencompact_pack()lazily rebuiltrepository.chunksafter the cached indexes had been invalidated.get_repository_chunks()passesinit_flags=F_NONE(found while reading the code: without it, a compact run that has to rebuild the index from the packs marks everything used and frees nothing).Both bugs have regression tests that fail on master: one drives create/delete/compact through a mixed pack rewrite and checks that the persisted index only references existing packs, that extract still works and that a second delete + compact completes; the other covers garbage collection after the cached index was lost.
Checklist
master(or maintenance branch if only applicable there)toxor the relevant test subset)